fix(mcp): add created_by_fk filter, field validators, and deduplicate defaults for database tools by aminghadersohi · Pull Request #39113 · apache/superset

aminghadersohi · 2026-04-04T20:44:24Z

Summary

Code review improvements for #39111:

Add created_by_fk and changed_by_fk to DatabaseFilter — Matches the chart/dashboard filter pattern so users can do "find my databases" queries via get_instance_info → list_databases(filters=[{"col": "created_by_fk", ...}])
Add field_validator for filters and select_columns — Handles JSON string parsing from MCP clients (double-serialization bug workaround), matching the pattern in DashboardFilter/ChartFilter
Remove duplicate DEFAULT_DATABASE_COLUMNS — Was defined in both schema_discovery.py and list_databases.py; now imports from the canonical location
Update app.py instructions — Document the database created_by_fk workflow in both "find your own" and "query examples" sections

Test plan

Existing unit tests still pass
list_databases with created_by_fk filter returns only databases created by the specified user
JSON string inputs for filters and select_columns are parsed correctly

… defaults for database tools - Add created_by_fk and changed_by_fk to DatabaseFilter columns so users can filter databases by creator (matching chart/dashboard patterns) - Add field_validator for filters and select_columns to handle JSON string parsing from MCP clients (double-serialization bug workaround) - Remove duplicate DEFAULT_DATABASE_COLUMNS from list_databases.py, import from schema_discovery.py instead - Update app.py instructions to document database created_by_fk workflow

bito-code-review · 2026-04-04T20:44:35Z

Bito Automatic Review Skipped - Branch Excluded

Bito didn't auto-review because the source or target branch is excluded from automatic reviews.
No action is needed if you didn't intend for the agent to review it. Otherwise, to manually trigger a review, type /review in a comment and save.
You can change the branch exclusion settings here, or contact your Bito workspace admin at evan@preset.io.

Copilot

Pull request overview

Improves the MCP database tools’ request ergonomics and filterability by aligning database listing schemas with existing chart/dashboard patterns, while reducing duplicated defaults.

Changes:

Added created_by_fk and changed_by_fk as supported filter columns for list_databases.
Added Pydantic field_validators to accept JSON-string inputs for filters and flexible inputs for select_columns.
Deduplicated database default columns by importing DATABASE_DEFAULT_COLUMNS from the schema discovery module and updated MCP default instructions with “my databases” examples.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
superset/mcp_service/database/tool/list_databases.py	Removes duplicate default column list and uses canonical `DATABASE_DEFAULT_COLUMNS`.
superset/mcp_service/database/schemas.py	Expands filter schema and adds validators to parse JSON-string/CSV inputs for MCP clients.
superset/mcp_service/app.py	Documents “find my databases” workflow using `created_by_fk` filtering.

Copilot · 2026-04-04T20:47:47Z

+    @field_validator("filters", mode="before")
+    @classmethod
+    def parse_filters(cls, v: Any) -> List[DatabaseFilter]:
+        """Accept both JSON string and list of objects."""
+        return parse_json_or_model_list(v, DatabaseFilter, "filters")
+
+    @field_validator("select_columns", mode="before")
+    @classmethod
+    def parse_columns(cls, v: Any) -> List[str]:
+        """Accept JSON array, list, or comma-separated string."""
+        return parse_json_or_list(v, "select_columns")


The new parsing behavior for filters and select_columns (JSON-string / CSV inputs) and the newly supported created_by_fk / changed_by_fk filter columns aren’t covered by unit tests. Please add tests (e.g., in tests/unit_tests/mcp_service/database/tool/test_database_tools.py) to verify: (1) JSON-string filters is accepted and converted into DatabaseFilter models, (2) JSON-string and comma-separated select_columns are accepted, and (3) created_by_fk filtering flows through to DatabaseDAO.list (assert the DAO is called with a ColumnOperator where col == 'created_by_fk').

codecov · 2026-04-04T20:48:15Z

Codecov Report

❌ Patch coverage is 53.84615% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.54%. Comparing base (81c4b12) to head (be7e1ab).
⚠️ Report is 2 commits behind head on mcp-database-tools.

Files with missing lines	Patch %	Lines
superset/mcp_service/database/schemas.py	53.84%	6 Missing ⚠️

Additional details and impacted files

@@                  Coverage Diff                   @@
##           mcp-database-tools   #39113      +/-   ##
======================================================
+ Coverage               64.52%   64.54%   +0.01%     
======================================================
  Files                    2540     2540              
  Lines                  131394   131403       +9     
  Branches                30466    30466              
======================================================
+ Hits                    84779    84810      +31     
+ Misses                  45152    45130      -22     
  Partials                 1463     1463

Flag	Coverage Δ
hive	`40.16% <53.84%> (+0.04%)`	⬆️
mysql	`60.92% <53.84%> (+0.04%)`	⬆️
postgres	`61.00% <53.84%> (+0.04%)`	⬆️
presto	`40.18% <53.84%> (+0.04%)`	⬆️
python	`62.59% <53.84%> (+0.04%)`	⬆️
sqlite	`60.63% <53.84%> (+0.04%)`	⬆️
unit	`100.00% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bito-code-review · 2026-04-04T20:49:15Z

To cover the new parsing behavior and filtering, add these unit tests to tests/unit_tests/mcp_service/database/tool/test_database_tools.py. They verify JSON-string filters conversion, select_columns parsing, and created_by_fk filter passing to DAO.

tests/unit_tests/mcp_service/database/tool/test_database_tools.py

import pytest
from unittest.mock import patch, MagicMock
from superset.mcp_service.database.schemas import ListDatabasesRequest
from superset.mcp_service.database.tool.list_databases import list_databases
from superset.mcp_service.common.context import Context


class TestListDatabasesRequest:
    def test_parse_filters_json_string(self):
        filters_json = '[{"col": "database_name", "opr": "eq", "value": "test"}]'
        request = ListDatabasesRequest(filters=filters_json)
        assert len(request.filters) == 1
        assert request.filters[0].col == "database_name"
        assert request.filters[0].opr == "eq"
        assert request.filters[0].value == "test"

    def test_parse_filters_list(self):
        filters_list = [{"col": "database_name", "opr": "eq", "value": "test"}]
        request = ListDatabasesRequest(filters=filters_list)
        assert len(request.filters) == 1
        assert request.filters[0].col == "database_name"

    def test_parse_select_columns_json_string(self):
        columns_json = '["id", "database_name"]'
        request = ListDatabasesRequest(select_columns=columns_json)
        assert request.select_columns == ["id", "database_name"]

    def test_parse_select_columns_comma_separated(self):
        columns_csv = "id,database_name"
        request = ListDatabasesRequest(select_columns=columns_csv)
        assert request.select_columns == ["id", "database_name"]

    def test_parse_select_columns_list(self):
        columns_list = ["id", "database_name"]
        request = ListDatabasesRequest(select_columns=columns_list)
        assert request.select_columns == ["id", "database_name"]


@pytest.mark.asyncio
class TestListDatabasesTool:
    @patch('superset.daos.database.DatabaseDAO.list')
    async def test_created_by_fk_filter_flows_to_dao(self, mock_list):
        mock_list.return_value = []
        filters = [{"col": "created_by_fk", "opr": "eq", "value": 1}]
        request = ListDatabasesRequest(filters=filters)
        ctx = MagicMock(spec=Context)
        await list_databases(request, ctx)
        mock_list.assert_called_once()
        call_args = mock_list.call_args
        assert any(f.col == "created_by_fk" for f in call_args[1]['filters'])

codeant-ai-for-open-source · 2026-04-04T20:49:27Z

            item_serializer=_serialize_database,
            filter_type=DatabaseFilter,
-            default_columns=DEFAULT_DATABASE_COLUMNS,
+            default_columns=DATABASE_DEFAULT_COLUMNS,


Suggestion: DATABASE_DEFAULT_COLUMNS includes changed_on_humanized but not changed_on; since the serializer computes humanized time from changed_on, default responses will produce changed_on_humanized=None for row-based DAO results. Ensure changed_on is also loaded when using default columns. [logic error]

Severity Level: Major ⚠️

- ❌ list_databases default response has changed_on_humanized always null. - ⚠️ MCP agents cannot display relative last-modified time.

Suggested change

default_columns=DATABASE_DEFAULT_COLUMNS,

default_columns=[*DATABASE_DEFAULT_COLUMNS, "changed_on"],

Steps of Reproduction ✅

1. Start Superset with MCP service enabled and call the `list_databases` FastMCP tool without providing `select_columns` (default usage), which hits `list_databases()` in `superset/mcp_service/database/tool/list_databases.py:56-63`. 2. Inside `list_databases`, a `ModelListCore` is constructed at `superset/mcp_service/database/tool/list_databases.py:110-121` with `default_columns=DATABASE_DEFAULT_COLUMNS` (`list_databases.py:115`), where `DATABASE_DEFAULT_COLUMNS` is defined as `["id", "database_name", "backend", "expose_in_sqllab", "changed_on_humanized"]` in `superset/mcp_service/common/schema_discovery.py:61-67`. 3. Because `ListDatabasesRequest.select_columns` has a default empty list (`superset/mcp_service/database/schemas.py:125-132`), `ModelListCore.run_tool` (`superset/mcp_service/mcp_core.py:83-91`) treats it as falsy and sets `columns_to_load = self.default_columns`, then calls `DatabaseDAO.list(..., columns=columns_to_load)` at `mcp_core.py:93-102`; `BaseDAO.list` (`superset/daos/base.py:25-49,74-86`) builds a SQLAlchemy query selecting only real column attributes (`id`, `database_name`, `backend`, `expose_in_sqllab`) and returns Row objects that do NOT include `changed_on`. 4. Each Row is passed to `serialize_database_object` via `_serialize_database` at `list_databases.py:103-107`, and `serialize_database_object` (`superset/mcp_service/database/schemas.py:233-268`) calls `getattr(database, "changed_on", None)` and `_humanize_timestamp(getattr(database, "changed_on", None))`; since the Row lacks `changed_on`, `getattr` returns `None`, `_humanize_timestamp` returns `None`, and both `changed_on` and `changed_on_humanized` end up as `None`. The final response is produced by `result.model_dump(context={"select_columns": columns_to_filter})` at `list_databases.py:145-154`, and `DatabaseInfo._filter_fields_by_context` (`database/schemas.py:61-76`) filters fields to the requested columns, which now include `"changed_on_humanized"` but not `"changed_on"`, so the client observes `changed_on_humanized: null` for every database in the default `list_databases` response.

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset/mcp_service/database/tool/list_databases.py **Line:** 115:115 **Comment:** *Logic Error: `DATABASE_DEFAULT_COLUMNS` includes `changed_on_humanized` but not `changed_on`; since the serializer computes humanized time from `changed_on`, default responses will produce `changed_on_humanized=None` for row-based DAO results. Ensure `changed_on` is also loaded when using default columns. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

👍 | 👎

- Fix datetime.now() without timezone in DatabaseError.create() and _humanize_timestamp() — use timezone-aware datetimes consistently - Add type hints to create_mock_database() test helper - Update mcp_core.py ModelGetSchemaCore docstring to include "database" in the model_type description

Copilot AI review requested due to automatic review settings April 4, 2026 20:44

pull-request-size Bot added the size/M label Apr 4, 2026

dosubot Bot added api Related to the REST API change:backend Requires changing the backend labels Apr 4, 2026

Copilot started reviewing on behalf of aminghadersohi April 4, 2026 20:45 View session

Copilot AI reviewed Apr 4, 2026

View reviewed changes

codeant-ai-for-open-source Bot reviewed Apr 4, 2026

View reviewed changes

github-actions Bot removed the api Related to the REST API label Apr 4, 2026

aminghadersohi merged commit be7e1ab into apache:mcp-database-tools Apr 4, 2026
58 checks passed

aminghadersohi deleted the amin/review-mcp-database-tools branch April 4, 2026 23:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(mcp): add created_by_fk filter, field validators, and deduplicate defaults for database tools#39113

fix(mcp): add created_by_fk filter, field validators, and deduplicate defaults for database tools#39113
aminghadersohi merged 2 commits into
apache:mcp-database-toolsfrom
aminghadersohi:amin/review-mcp-database-tools

aminghadersohi commented Apr 4, 2026

Uh oh!

bito-code-review Bot commented Apr 4, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 4, 2026

Uh oh!

codecov Bot commented Apr 4, 2026 •

edited

Loading

Uh oh!

bito-code-review Bot commented Apr 4, 2026

Uh oh!

codeant-ai-for-open-source Bot Apr 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	default_columns=DATABASE_DEFAULT_COLUMNS,
	default_columns=[*DATABASE_DEFAULT_COLUMNS, "changed_on"],

Uh oh!

Conversation

aminghadersohi commented Apr 4, 2026

Summary

Test plan

Uh oh!

bito-code-review Bot commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bito-code-review Bot commented Apr 4, 2026

Uh oh!

codeant-ai-for-open-source Bot Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bito-code-review Bot commented Apr 4, 2026 •

edited

Loading

codecov Bot commented Apr 4, 2026 •

edited

Loading